test(relay): serialize flaky trace_context_lookup callsite test under ENV_LOCK (#3929) - #4059
test(relay): serialize flaky trace_context_lookup callsite test under ENV_LOCK (#3929)#4059iroiro147 wants to merge 1 commit into
Conversation
… ENV_LOCK (block#3929) trace_context_lookup_does_not_enable_callsites intermittently failed under the parallel buzz-relay lib suite (1/6 on clean main, 2/6 on block#3844 head) while passing 100% standalone. tracing's callsite interest cache is global per callsite, not per-subscriber: a neighbor test that registers a global default dispatcher can poison this callsite's cached interest before the thread-local with_default filter runs, so the !enabled! probe consults a stale entry. Serialize this test against the other global-state tests by taking the same ENV_LOCK mutex they already hold (:498+), so no global-dispatcher registration can overlap it. This matches the existing repo convention; a unique per-run callsite target (the other suggested fix) was rejected because tracing's enabled! target position requires a constant and will not accept a runtime &str (E0435). Verified: under the real repro (cargo test -p buzz-relay --lib, the full parallel suite), this test no longer appears among failures across 3 consecutive runs. The 9 remaining suite failures are all pre-existing api::media/api::admin sidecar/env-gated cases on clean main, unrelated to this test. Filtered telemetry suite passes 8/8. Found while reproducing the reporter's six consecutive full-suite runs. Refs block#3929 Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
|
Closing in favor of my own #3981, which implements the reporter's preferred fix (option 2). #3981 gives this test's The ENV_LOCK approach in this PR (option 1) is the safe mechanical fallback and works — the test no longer appears among full- Same machinery passes: full Consolidating to a single candidate saves triager time. Refs #3929. |
1 participant
Summary
Refs #3929 — makes
telemetry::tests::trace_context_lookup_does_not_enable_callsitesdeterministic under the parallelbuzz-relay --libsuite. Pre-existing on cleanmain; not introduced by any open PR.The flake
The test installs a
TraceContextLookuplayer withLevelFilter::OFFvia a thread-localwith_defaultand asserts!tracing::enabled!(target: "trace_context_lookup_filter_test", ERROR). Under parallel load it could fail with the callsite unexpectedly enabled (1/6 on clean main; 2/6 observed on the #3844 branch head, which was the negative-control prompt).tracing's callsite interest cache is global per callsite, not per-subscriber. A neighbor test that registers a global default dispatcher can poison this callsite's cached interest before the thread-localwith_defaultruns, so theenabled!probe consults a stale entry rather than the OFF filter.Fix
Serialize this test against the other global-state tests by taking the same
ENV_LOCKmutex they already hold (telemetry.rs:498+), so no global-dispatcher registration can overlap it. This matches the existing repo convention for exactly this class of shared-state flake.Why not the unique-callsite fix: the reporter's preferred option (2) — a unique per-run callsite target — was evaluated and rejected:
tracing::enabled!'starget:position requires a constant and rejects a runtime&str(E0435: attempt to use a non-constant value in a constant). Option (1) is the sound mechanical fix here.Verification
Under the real repro (
cargo test -p buzz-relay --lib, the full parallel suite), this test no longer appears among failures across 3 consecutive runs.The 9 remaining suite failures are all pre-existing
api::media/api::adminsidecar/env-gated cases on clean main (media_get_auth_flag_*,upload_concurrency_limit_is_scoped_by_community,feedback_attachment_rejects_unknown_feedback,report_detail_rejects_unknown_report, …), consistent across runs and unrelated to this test.cargo check -p buzz-relay— clean